[SPARK-59401][SQL] Read INT96 timestamp columns as nanosecond timestamps - #58693
[SPARK-59401][SQL] Read INT96 timestamp columns as nanosecond timestamps#58693stevomitric wants to merge 3 commits into
Conversation
### What changes were proposed in this pull request? Extend the nanosecond-timestamp Parquet read path (which already reads INT64 TIMESTAMP(MICROS)) to also read a legacy INT96 timestamp column as a nanosecond type -- the read side of widening a TIMESTAMP(6) column stored as INT96 to nanosecond precision. INT96 carries no logical unit and decodes to microseconds (ParquetRowConverter.binaryToSQLTimestamp), so each value is promoted to the internal (epochMicros, nanosWithinMicro = 0) representation -- the same promotion the MICROS path uses. The requested type's family decides handling, mirroring the INT96 arms of ParquetRowConverter: the LTZ family applies the INT96 Julian rebase and any timezone conversion; the NTZ family applies neither. INT96 spans the full date range, so the promotion is range-complete. - Row-based: TimestampNanosParquetOps.newConverter accepts an INT96 column via a new int96AsNanosConverter (guarded by isInt96Timestamp). - Vectorized: a new Int96AsTimestampNanosUpdater, dispatched for a nanos type over an INT96 column. ### Why are the changes needed? INT96 is the default on-disk encoding for LTZ timestamps, so without this an INT96-encoded TIMESTAMP(6) column could not be widened to nanoseconds and read back. This complements the INT64 TIMESTAMP(MICROS) read support. ### Does this PR introduce _any_ user-facing change? Yes, behind the preview flag spark.sql.timestampNanosTypes.enabled: reading an INT96 timestamp column under a nanosecond timestamp type now succeeds (previously threw). ### How was this patch tested? ParquetTypeWideningSuite now covers INT96 -> LTZ nanos (moved from the unsupported list; TIMESTAMP_MILLIS stays unsupported), across vectorized/row-based readers and dictionary on/off. ### Was this patch authored or co-authored using generative AI tooling? Co-authored-by: Isaac <no-reply@databricks.com>
uros-b
left a comment
There was a problem hiding this comment.
[WIP][SPARK-56822][SQL] Read INT96 timestamp columns as nanosecond timestamps
Are we ready drop WIP here?
Also, SPARK-56822 doesn't look right here. Should it be SPARK-59284?
…/NTZ reads Address review feedback on reading INT96 timestamp columns as a nanosecond type. INT96 stores nanoseconds-of-day, so a foreign file (e.g. Impala/Hive) can carry true sub-microsecond digits. binaryToSQLTimestamp floors to micros, so the read now recovers the remainder straight from the raw INT96 (timeOfDayNanos % 1000) and truncates it to the read precision via the shared DateTimeUtils.truncateNanosWithinMicroToPrecision, instead of hardcoding nanosWithinMicro = 0. A whole-microsecond rebase/timezone shift never perturbs the remainder. Applied to both readers; the vectorized updater now carries the read precision. Other changes: - Build the INT96 rebase closure only for the LTZ family (identity for NTZ), matching microsAsNanosConverter. - Clarify that INT96 has no time-zone family, so it can be requested as either LTZ or NTZ nanos (mirroring Spark's existing INT96 -> TimestampType / TimestampNTZType reads); the same-family guard applies only to the annotated micros path. Comments no longer claim INT96 has no sub-microsecond digits. Tests: - ParquetTypeWideningSuite now runs the INT96 read under both CORRECTED and LEGACY rebase over a pre-1582 value, keeping dictionary on/off. - TimestampNanosParquetOpsSuite gains INT96 unit tests: sub-microsecond preservation (both families), precision truncation, LTZ rebase (LEGACY differs from CORRECTED, EXCEPTION throws) and timezone conversion, and NTZ ignoring both. Co-authored-by: Isaac <no-reply@databricks.com>
uros-b
left a comment
There was a problem hiding this comment.
The core change looks correct: the vectorized Int96AsTimestampNanosUpdater LTZ arm reproduces the existing INT96→Timestamp conversion tree, the NTZ arm matches BinaryToSQLTimestampUpdater, and rebaseInt96 is equivalent to the row path's rebase across CORRECTED/LEGACY/EXCEPTION. What's missing is test coverage on the vectorized path specifically — the end-to-end checkAllParquetReaders case only writes micro-aligned INT96 through Spark's own writer (so the timeOfDayNanos % 1000 remainder is always 0), is LTZ-only, and runs only CORRECTED+LEGACY for the nanos target types. So the headline behavior (a foreign, genuinely sub-microsecond INT96 recovered as nanos), the NTZ arm, and the EXCEPTION (failIfRebase) arm are all pinned only on the row converter, never exercised vectorized. Could you add a vectorized case with a real sub-µs INT96 (a directly-written/foreign file, or a fixture that isn't micro-aligned) covering NTZ and the EXCEPTION rebase mode? The logic reads right — it's the coverage of the exact new behavior that's the gap.
Add end-to-end tests that drive the vectorized Int96AsTimestampNanosUpdater (putInt96AsNanos) with a hand-crafted foreign INT96 file, since Spark only writes micro-aligned INT96 and the widening suite therefore never yields a non-zero sub-microsecond remainder through the default reader: - "INT96 vectorized read preserves sub-microsecond nanos from a foreign file": reads a forged foreign INT96 (1970-01-01 12:34:56.123456789) as both the NTZ and LTZ families under withAllParquetReaders, dictionary on/off, so the sub-micro recovery and the dictionary-decode branch are both exercised. - "INT96 read of an ancient value fails under EXCEPTION rebase for LTZ": drives the failIfRebase arm end to end (the vectorized reader throws SparkUpgradeException directly; the row-based reader wraps it in SparkException). The row-based decodeInt96 unit tests in TimestampNanosParquetOpsSuite stay as the complement. Co-authored-by: Isaac <no-reply@databricks.com>
|
Fixed now, added a vectorized-reader test that reads a foreign INT96 carrying sub-microsecond digits. It covers both NTZ and LTZ, dictionary on/off, plus the EXCEPTION-rebase case. |
What changes were proposed in this pull request?
Extend the nanosecond-timestamp Parquet read path (which already reads INT64 TIMESTAMP(MICROS)) to also read a legacy INT96 timestamp column as a nanosecond type - the read side of widening a TIMESTAMP(6) column stored as INT96 to nanosecond precision.
INT96 carries no logical unit and decodes to microseconds (ParquetRowConverter.binaryToSQLTimestamp), so each value is promoted to the internal (epochMicros, nanosWithinMicro = 0) representation - the same promotion the MICROS path uses. The requested type's family decides handling, mirroring the INT96 arms of ParquetRowConverter: the LTZ family applies the INT96 Julian rebase and any timezone conversion; the NTZ family applies neither. INT96 spans the full date range, so the promotion is range-complete.
Why are the changes needed?
INT96 is the default on-disk encoding for LTZ timestamps, so without this an INT96-encoded TIMESTAMP(6) column could not be widened to nanoseconds and read back. This complements the INT64 TIMESTAMP(MICROS) read support.
Does this PR introduce any user-facing change?
Yes, behind the preview flag spark.sql.timestampNanosTypes.enabled, reading an INT96 timestamp column under a nanosecond timestamp type now succeeds (previously threw).
How was this patch tested?
extended ParquetTypeWideningSuite.
Was this patch authored or co-authored using generative AI tooling?
Co-authored-by: Claude Code 4.8